Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented May 14, 2025

What do these changes do?

Allows to define services with the new LegacyState attribute inside PathMappingsLabel which maps the state from the legacy version to the state in the new style version of the service.

Example Jupyter FEniCS:

  • legacy workspace
    Screenshot 2025-05-16 at 09 57 59

  • new style workspace (pleas note the inputs and output have been copied over)
    Screenshot 2025-05-16 at 09 59 18

Related issue/s

How to test

Dev-ops

@GitHK GitHK self-assigned this May 14, 2025
@codecov
Copy link

codecov bot commented May 14, 2025

Codecov Report

Attention: Patch coverage is 72.15190% with 22 lines in your changes missing coverage. Please review.

Project coverage is 88.96%. Comparing base (3595349) to head (9f266a5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7675      +/-   ##
==========================================
+ Coverage   87.53%   88.96%   +1.43%     
==========================================
  Files        1811     1453     -358     
  Lines       70260    60117   -10143     
  Branches     1137      470     -667     
==========================================
- Hits        61500    53483    -8017     
+ Misses       8451     6514    -1937     
+ Partials      309      120     -189     
Flag Coverage Δ
integrationtests 64.41% <32.25%> (+0.04%) ⬆️
unittests 88.08% <72.15%> (+1.32%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library 93.06% <93.75%> (-0.03%) ⬇️
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration 69.92% <ø> (ø)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.07% <15.00%> (-0.54%) ⬇️
agent 96.46% <ø> (ø)
api_server 91.62% <ø> (ø)
autoscaling 96.08% <ø> (ø)
catalog 92.64% <ø> (ø)
clusters_keeper 99.25% <ø> (ø)
dask_sidecar 89.86% <ø> (ø)
datcore_adapter 98.12% <ø> (ø)
director 76.80% <ø> (ø)
director_v2 91.12% <100.00%> (-0.03%) ⬇️
dynamic_scheduler 96.76% <ø> (ø)
dynamic_sidecar 90.18% <77.77%> (-0.04%) ⬇️
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
payments 92.63% <ø> (ø)
resource_usage_tracker 89.18% <ø> (+0.16%) ⬆️
storage 87.63% <ø> (-0.11%) ⬇️
webclient ∅ <ø> (∅)
webserver 85.82% <ø> (+<0.01%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3595349...9f266a5. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GitHK GitHK changed the title 🐛 fixed issue with get_image_name_and_tag dynamic-sidecar is now able to extract the state coming from a different archive when upgrading from a legacy service May 16, 2025
@GitHK GitHK changed the title dynamic-sidecar is now able to extract the state coming from a different archive when upgrading from a legacy service dynamic-sidecar allows to import old state from legacy services May 16, 2025
@GitHK GitHK changed the title dynamic-sidecar allows to import old state from legacy services dynamic-sidecar allows to import old state from legacy services 🚨 May 16, 2025
Andrei Neagu added 2 commits May 16, 2025 10:13
@GitHK GitHK requested a review from Copilot May 16, 2025 08:16
@GitHK GitHK added a:dynamic-sidecar dynamic-sidecar service a:simcore-sdk labels May 16, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces support for importing and migrating legacy service state by adding a new LegacyState model, integrating it into configuration, workflows, and environment specs.

  • Adds a LegacyState Pydantic model and validation in PathMappingsLabel
  • Extends dynamic-sidecar settings and long-running tasks to handle legacy state paths
  • Propagates DY_SIDECAR_LEGACY_STATE through Docker environment variables and SDK data manager operations

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
services/dynamic-sidecar/.../core/settings.py Adds DY_SIDECAR_LEGACY_STATE configuration field
services/dynamic-sidecar/.../modules/long_running_tasks.py Implements _get_legacy_state_with_dy_volumes_path and passes legacy_state to pull/push
services/director-v2/.../docker_service_specs/sidecar.py Exposes DY_SIDECAR_LEGACY_STATE as an environment variable
services/director-v2/tests/unit/.../test_modules_dynamic_sidecar_docker_service_specs.py Updates expected spec to include "DY_SIDECAR_LEGACY_STATE": "null"
services/director-v2/tests/unit/.../test_modules_dynamic_sidecar_docker_service_specs_sidecar.py Adds DY_SIDECAR_LEGACY_STATE to the env var list in sidecar spec tests
packages/simcore-sdk/src/simcore_sdk/node_data/data_manager.py Adds legacy_state parameters to pull and push functions
packages/simcore-sdk/tests/unit/test_node_data_data_manager.py Parametrizes test_pull_legacy_archive to cover legacy archives
packages/models-library/src/models_library/service_settings_labels.py Introduces LegacyState model, integrates into PathMappingsLabel, and adds a validator
Comments suppressed due to low confidence (1)

packages/simcore-sdk/src/simcore_sdk/node_data/data_manager.py:199

  • There are no unit tests covering the new legacy_destination_path code path in _pull_legacy_archive or the legacy_state branch in push/pull. Consider adding tests for both legacy_state present/absent scenarios.
async def _pull_legacy_archive(

@GitHK GitHK marked this pull request as ready for review May 16, 2025 08:26
@GitHK GitHK added this to the Bazinga! milestone May 16, 2025
@GitHK GitHK requested a review from mguidon May 16, 2025 09:17
Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx

@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label May 16, 2025
@GitHK
Copy link
Contributor Author

GitHK commented May 16, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented May 16, 2025

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details about the failure.

@mergify
Copy link
Contributor

mergify bot commented May 16, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@GitHK
Copy link
Contributor Author

GitHK commented May 19, 2025

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented May 19, 2025

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request.

@mergify
Copy link
Contributor

mergify bot commented May 19, 2025

queue

🛑 The pull request has been removed from the queue default

The following conditions don't match anymore:

  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • any of: [🛡 GitHub branch protection]
        • check-neutral = deploy to dockerhub
        • check-skipped = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-neutral = system-tests
        • check-skipped = system-tests
        • check-success = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-neutral = unit-tests
        • check-skipped = unit-tests
        • check-success = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-neutral = integration-tests
        • check-skipped = integration-tests
        • check-success = integration-tests

@sonarqubecloud
Copy link

@mergify
Copy link
Contributor

mergify bot commented May 19, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@GitHK
Copy link
Contributor Author

GitHK commented May 19, 2025

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented May 19, 2025

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request.

@mergify
Copy link
Contributor

mergify bot commented May 19, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c8bbdc2

@mergify mergify bot merged commit c8bbdc2 into ITISFoundation:master May 19, 2025
94 of 96 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jun 6, 2025
92 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:dynamic-sidecar dynamic-sidecar service a:simcore-sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants